Add StringWriter#10333
Conversation
treblereel
force-pushed
the
string_writer
branch
2 times, most recently
from
af9b171 to
1df014e
Compare
- added a GWT port of StringWriter for compatibility with the J2CL Java emulation layer Signed-off-by: Dmitrii Tikhomirov <chani.liet@gmail.com>
|
hmm, not sure if it's related to this pr
|
niloc132
left a comment
niloc132
left a comment
There was a problem hiding this comment.
I do see that j2cl also has ASF sources for this class, contributed in google/j2cl@7a98b9c, but with slightly different contents, and fairly different tests (so I don't think you're attributing Google because you copied it from them?).
| @@ -0,0 +1,141 @@ | |||
| /* | |||
| * Copyright 2025 Google Inc. | |||
| public void testConstructor() { | ||
| assertTrue("Used in tests", true); | ||
| } |
There was a problem hiding this comment.
why include the test if it doesnt do anything?
| @@ -0,0 +1,96 @@ | |||
| // CHECKSTYLE_OFF: Copyrighted to the Android Open Source Project. | |||
| /* | |||
| * Licensed to the Apache Software Foundation (ASF) under one or more | |||
There was a problem hiding this comment.
While I definitely appreciate the attribution and the fact that there is an apache licensed emul out there we can lean on, two questions:
- first, this appears simple enough it could just be implemented from scratch, right? the test apparently isnt from asf, so that work had to be done fresh...?
- second, who is the copyright owner? my read of the copyright notice is that we must list the owner. The content at https://www.apache.org/licenses/LICENSE-2.0.html (almost the link below, but https) says
To apply the Apache License to specific files in your work, attach the following boilerplate declaration, replacing the fields enclosed by brackets "[]" with your own identifying information. (Don't include the brackets!) Enclose the text in the appropriate comment syntax for the file format. We also recommend that you include a file or class name and description of purpose on the same "printed page" as the copyright notice for easier identification within third-party archives.
Copyright [yyyy] [name of copyright owner]
From https://www.apache.org/foundation/license-faq,
Note that the Apache Software Foundation uses a different source header that is related to our use of a CLA. Our instructions for our project's source headers are here.
niloc132
added
the
ready
Signed-off-by: Dmitrii Tikhomirov <chani.liet@gmail.com>
| import static javaemul.internal.InternalPreconditions.checkArgument; | ||
|
|
||
| /** | ||
| * See <a href="http://java.sun.com/j2se/1.5.0/docs/api/java/io/StringWriter.html">the official |
There was a problem hiding this comment.
| * See <a href="http://java.sun.com/j2se/1.5.0/docs/api/java/io/StringWriter.html">the official | |
| * See <a href="https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/io/StringWriter.html">the official |
Maybe latest stable LTS? Alternatively 17 would also make some sense as it's currently the highest possible value of source level for GWT.
There was a problem hiding this comment.
On the other hand, its supersource, so the package and classname is really all we need... More important would be any notes about what makes the GWT impl unique (though this class won't have much of that).
There was a problem hiding this comment.
this class was taken from j2cl and adapted for gwt. In J2CL, the reference points to java.sun.com (which redirects to Oracle)
But I think that's probably the least significant of the issues that could exist
There was a problem hiding this comment.
Yeah I worry about the licensing because it does matter, but the code looks good to me.
If the tests are from google though, they should be attributed as such rather than replace the owner field.
Signed-off-by: Dmitrii Tikhomirov <chani.liet@gmail.com>
3 participants
ref https://github.com/google/j2cl/blob/master/jre/java/java/io/StringWriter.java